docs: ADR 0050 — distributed tracing instrumentation with OpenTelemetry - #1489
Conversation
cadc284 to
c11a17b
Compare
ralphbean
left a comment
ralphbean
left a comment
There was a problem hiding this comment.
A couple things to sort out — see inline.
c11a17b to
2c1189b
Compare
Revision: ADR refactored and narrowedBased on team discussion (June 11 sync) and reviewer feedback, this PR has been substantially refactored: What changed
What was removed from the ADR (→ belongs elsewhere)
These remain documented in the implementation reference ( Reviewer feedback addressedAll |
E2E tests did not runE2E tests run automatically for org/repo members and collaborators on pull requests. For other contributors, a maintainer must add the See E2E testing guide for details. |
|
🤖 Review · |
2c1189b to
ac20880
Compare
ascerra
changed the title
Site previewPreview: https://5a69e356-site.fullsend-ai.workers.dev Commit: |
|
🤖 Review · |
ac20880 to
8dcd48b
Compare
ascerra
changed the title
8dcd48b to
640d2c9
Compare
|
🤖 Finished Review · ✅ Success · Started 7:30 PM UTC · Completed 7:45 PM UTC |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
ReviewFindingsLow
Info
Previous runReviewFindingsCritical
Low
Info
Labels: PR adds a new ADR and infrastructure guide — purely documentation changes. Previous run (2)ReviewFindingsLow
Info
Previous run (3)ReviewFindingsLow
Info
Previous run (4)ReviewFindingsMedium
Low
Info
Previous run (5)ReviewFindingsLow
Info
Previous run (6)ReviewFindingsMedium
Low
Info
Previous run (7)ReviewFindingsMedium
Low
Info
Previous run (8)ReviewFindingsMedium
Low
Info
Previous run (9)ReviewFindingsLow
Info
Previous run (10)ReviewFindingsLow
Info
Previous run (11)ReviewFindingsMedium
Low
Info
Previous run (12)ReviewFindingsLow
Info
Previous run (13)ReviewFindingsMedium
Low
Info
Previous run (14)ReviewFindingsMedium
Low
Info
Previous run (15)ReviewFindingsMedium
Low
Info
|
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
| - opentelemetry | ||
| --- | ||
|
|
||
| # 46. Framework-native distributed tracing with OpenTelemetry |
There was a problem hiding this comment.
[low] internal-consistency
YAML front-matter title says 47 but the markdown heading says 46 — should be 47 to match.
Suggested fix: Change line 12 from # 46. to # 47.
| appropriate for the content sensitivity | ||
| - Enables LLM-judge evaluation scorers that need to read agent reasoning | ||
|
|
||
| **Additional design properties:** |
There was a problem hiding this comment.
[low] internal-consistency
ADR claims multi-endpoint support but the guide only documents single-endpoint env vars per the OTEL specification.
Suggested fix: Either remove the multi-endpoint claim, document the mechanism in the guide, or clarify that multi-backend export requires an OTEL Collector.
There was a problem hiding this comment.
Noted, non-blocking. I think we should be explicit that multi-backend export needs an OTEL Collector as a fan-out proxy — the standard env vars take a single URL.
|
|
||
| Setting `FULLSEND_TELEMETRY=1` enables telemetry recording even without an | ||
| OTLP endpoint (local files only, useful for testing). | ||
|
|
There was a problem hiding this comment.
[low] edge-case
FULLSEND_TELEMETRY=1 purpose is ambiguous relative to the ADR zero-config Level 1 baseline that always produces local files.
Suggested fix: Clarify what FULLSEND_TELEMETRY=1 enables beyond the zero-config baseline, or remove if redundant.
|
|
||
| **Open questions:** | ||
|
|
||
| - What signals matter most — cost, latency, token usage, action logs, decision traces, or something else? |
There was a problem hiding this comment.
[info] scope-verification
The tracing-vs-volume question is struck through in architecture.md but the related granularity question in operational-observability.md remains open. Internally consistent but worth noting.
fullsend-ai-review
Bot
added
the
requires-manual-review
ralphbean
left a comment
ralphbean
left a comment
There was a problem hiding this comment.
Prior feedback addressed. +1'd a few of the bot's inline notes (non-blocking).
640d2c9 to
fd20d36
Compare
ascerra
changed the title
fullsend-ai-review
Bot
added
ready-for-merge
I think all of Marta's concerns were addressed. I want to get this in, so that #2423 can be rebased on it.
|
fwiw, I think implementation of this ADR is lower priority than the feature refinement work. |
|
🤖 Finished Review · ✅ Success · Started 6:05 PM UTC · Completed 6:20 PM UTC |
| @@ -0,0 +1,143 @@ | |||
| --- | |||
There was a problem hiding this comment.
[critical] naming-collision
ADR number 0049 is already taken. The repository contains docs/ADRs/0049-agent-configuration-env-var-convention.md (accepted, dated 2026-06-16) on main. This PR introduces a second, unrelated ADR 0049. All cross-references to ADR 0049 throughout the repository (currently docs/architecture.md:98 references the env-var convention ADR) become ambiguous. When this PR merges, the repo will contain two files with the same ADR number. The next available number is 0050.
Suggested fix: Renumber the distributed tracing ADR to 0050. Update all references introduced by this PR: docs/ADRs/0021-jsonl-reasoning-trace-exposure.md, docs/architecture.md, docs/guides/infrastructure/distributed-tracing.md, docs/problems/operational-observability.md, and the ADR own title, heading, and filename.
fullsend-ai-review
Bot
added
component/docs
Add ADR recording the decision to instrument fullsend with OpenTelemetry using a three-level opt-in model (local files → OTLP metadata export → content capture). Separates telemetry from evaluation concerns. Key changes: - ADR 0048: three-level content sensitivity model per OTEL GenAI spec, explicit scope boundary (evals consume traces, separate concern), multi-backend via OTEL Collector (not multi-endpoint config) - Infrastructure guide: env var precedence, local dev section, content capture warning; backend-agnostic language throughout - Cross-reference annotation in ADR 0021 (OTel future → now decided) - Update cross-references in architecture.md and problem doc Addresses review feedback from ralphbean, maruiz93, and review bot. Signed-off-by: Adam Scerra <ascerra@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: Adam Scerra <ascerra@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: Adam Scerra <ascerra@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: Adam Scerra <ascerra@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: Adam Scerra <ascerra@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: Adam Scerra <ascerra@redhat.com> Co-authored-by: Cursor <cursoragent@cursor.com>
badd2c5 to
a777a5d
Compare
ascerra
changed the title
|
🤖 Finished Review · ✅ Success · Started 6:47 PM UTC · Completed 7:00 PM UTC |
| human debugging). Complementary structured extraction via OpenTelemetry | ||
| could power aggregate analysis at scale (pattern detection across many | ||
| runs) — a future decision, not in scope here. | ||
| runs) — subsequently decided in [ADR 0050](0050-distributed-tracing-instrumentation.md). |
There was a problem hiding this comment.
[low] scope-coherence
Edit to Accepted ADR 0021 replaces a future decision, not in scope here with subsequently decided in [ADR 0050]. Permitted cross-reference annotation but replaces original scoping framing rather than appending.
Suggested fix: Consider rewording to: a future decision, not in scope here — subsequently decided in [ADR 0050].
| @@ -0,0 +1,143 @@ | |||
| --- | |||
There was a problem hiding this comment.
[low] ADR-frontmatter-consistency
The ADR template (0000) includes an HTML comment block after the Status section explaining ADR mutability policy. ADR 0050 omits this comment.
|
|
||
| **Open questions:** | ||
|
|
||
| - What signals matter most — cost, latency, token usage, action logs, decision traces, or something else? |
There was a problem hiding this comment.
[info] internal-consistency
Struck-through question resolution states volume is managed by backends not by suppressing data at the source. Accurate for span volume but readers may conflate volume with content.
fullsend-ai-review
Bot
added
the
ready-for-merge
|
🤖 Finished Retro · ✅ Success · Started 7:26 PM UTC · Completed 7:34 PM UTC |
Retro: PR #1489 — ADR 0050 (distributed tracing with OpenTelemetry)Workflow: Human-authored docs PR by @ascerra, open 23 days (2026-05-26 to 2026-06-18). Received substantive review from two humans (@ralphbean, @maruiz93) and 16 review bot runs (3 cancelled, 1 failure, 12 successful). What went well
Patterns observed (all covered by existing issues)
AssessmentNo novel proposals warranted. The existing backlog comprehensively covers the improvement areas this PR surfaced. The most impactful existing issues for this class of PR are #992 (CI ADR collision lint), #1013 (finding deduplication), and #2200 (missing-authorization severity cap for human PRs). Prioritizing those three would address the main friction points observed here. |
3 participants
Summary
Adds ADR 0048 recording the architectural decision to instrument fullsend with OpenTelemetry using a three-level opt-in model. Includes an infrastructure guide for operators.
What's in this PR
docs/ADRs/0048-distributed-tracing-instrumentation.md— the ADR, focused on the single decision: how fullsend produces tracesdocs/guides/infrastructure/distributed-tracing.md— operator guide covering configuration, env var precedence, local dev setup, and content capture warningsarchitecture.md,operational-observability.md, and ADR 0021Three-level opt-in model (the core decision)
run-events.jsonl,run-summary.json(metadata only)OTEL_EXPORTER_OTLP_ENDPOINTsetOTEL_INSTRUMENTATION_GENAI_CAPTURE_MESSAGE_CONTENT=trueWhat this ADR does NOT decide
Review bot findings addressed
FULLSEND_TELEMETRY=1ambiguity → clarified (needed outside GHA for structured recording)Addresses reviewer feedback from